otel: add test for batch splitting with the elasticsearchexporter#6334
otel: add test for batch splitting with the elasticsearchexporter#6334mauri870 wants to merge 1 commit intoelastic:mainfrom
Conversation
mauri870
added
Team:Elastic-Agent-Data-Plane
mauri870
requested review from
blakerouse,
faec,
michalpristas and
rdner
and removed request for
a team
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
|
This pull request does not have a backport label. Could you fix it @mauri870? 🙏
|
|
|
mergify
bot
added
the
backport-8.x
mauri870
force-pushed
the
otel-test-batcher-entity-too-large
branch
from
d42229b to
3519ee7
Compare
|
|
@mauri870 does this PR fixes https://github.com/elastic/ingest-dev/issues/3677? After reading the description I am not sure what the |
| // This test asserts that the batcher configuration from the elasticseachexporter | ||
| // splits batches to avoid a 413 Request Entity Too Large error from Elasticsearch. | ||
| info := define.Require(t, define.Requirements{ |
There was a problem hiding this comment.
Just double checking why we need this to be an integration test that runs the entire collector. What additional test coverage is this adding? Why can't this be covered by a unit test in the exporter?
I like integration tests, they catch a lot of problems, but they are also driving our CI execution time to infinity :)
There was a problem hiding this comment.
Agree, I think this is better suited to be in the exporter. Actually, it might as well be covered in the exporter already by open-telemetry/opentelemetry-collector-contrib#36396. I will double check.
"For" in this context means "Relates to" that issue. Unfortunately the issue is an umbrella to different corner cases that need to be covered, and this PR is a test for one of them. |
|
So I looked into the tests and the elasticsearchexporter has coverage of test options, including flush::interval and flush::bytes here, I don't think an integration test in this case adds any additional coverage. |
4 participants
What does this PR do?
Adds a test that the elasticsearchexporter does batch splitting properly on large batches comming from a beats receiver.
Why is it important?
This guarantees that the scenario covered in elastic/beats#41911 does not happen again.
Checklist
./changelog/fragmentsusing the changelog toolRelated issues